Wip/hughsie/atmel flip #292
Conversation
| DfuTargetTransferFlags flags, | ||
| GCancellable *cancellable, | ||
| GError **error); | ||
| }; |
There was a problem hiding this comment.
This may not be a large issue, and intended but i'll mention in case it's an oversight.
You previously had padding for 8 reservations but only added 6 function assignments.
There was a problem hiding this comment.
Good point, but I removed the padding as we're not installing the library I don't care about ABI much. If you're more comfortable I can remove all the class padding in a separate commit.
There was a problem hiding this comment.
No worry, I think it's fine as is. just wanted to make sure you were aware. Since it's internal private ABI probably not a big deal.
There was a problem hiding this comment.
I've removed this manually in master.
| @@ -689,6 +710,10 @@ dfu_device_set_quirks (DfuDevice *device) | |||
| } | |||
| } | |||
|
|
|||
| /* Atmel bootloader */ | |||
| if (vid == 0x03eb) | |||
There was a problem hiding this comment.
It would be good to put this into a #define for the vendor instead of comment.
There was a problem hiding this comment.
I've avoided this in the set_quirks() function as we only need it once, and I'm not sure it adds to the readability.
There was a problem hiding this comment.
Well maybe not now, but i was thinking if the quirks grows a lot then it would make it more readable later to do it that way.
There was a problem hiding this comment.
I think this is solved by #293 :)
plugins/dfu/dfu-target-stm.c
Outdated
| /* for ST devices, the action only occurs when we do GetStatus */ | ||
| return dfu_target_check_status (target, cancellable, error); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Why is this stuff #if'ed out? If it's going to stick around, at least indicate what the TODO is to restore it.
There was a problem hiding this comment.
I guess it can go completely, we have git history in case it's ever required. Will Fix.
plugins/dfu/dfu-common.h
Outdated
| @@ -171,6 +172,7 @@ typedef enum { | |||
| DFU_VERSION_DFU_1_0 = 0x0100, | |||
| DFU_VERSION_DFU_1_1 = 0x0110, | |||
| DFU_VERSION_DFUSE = 0x011a, | |||
There was a problem hiding this comment.
So shouldn't this be switched to DFU_VERSION_STM?
There was a problem hiding this comment.
I flip-flopped on this. STM do actual call the version number DfuSe, and it's a defined value. I think adding a comment will make this clearer.
hughsie
force-pushed
the
wip/hughsie/atmel-flip
branch
from
9a66689
to
06f6a3c
Compare
This allows us to show the user something they can include in a bug report.
This allows us to fail optional things and still show useful output in: $ dfu-tool list
In the future we'll use this to set vendor-specific quirks.
This allows us to segment a buffer taking into account page borders and maximum transfer sizes.
hughsie
force-pushed
the
wip/hughsie/atmel-flip
branch
from
06f6a3c
to
586dc39
Compare
|
I've added two more commits, if those look good I think we can merge this. |
Currently the only supported model is the AT32UC3A3256S, more to come...
When the AVR device is not recognised as a supported part we can't read or write the firmware from the chip.
Guess the file type if it is not explicitly specified.
Also, fix the checksum to actually produce valid files...
hughsie
force-pushed
the
wip/hughsie/atmel-flip
branch
from
586dc39
to
81e0e00
Compare
No description provided.